Double pinyin query #2427
Merged
Double pinyin query #2427
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
taooceros
reviewed
Dec 28, 2023
VictoriousRaptor
force-pushed
the
double-pin
branch
from
73c4a69 to
22f6ad7
Compare
|
@VictoriousRaptor what's the status of this pr? |
POC. I'm using it daily and so far so good. Since there're many double pinyin mappings we should make it user configurable. Is it acceptable to allow users to edit a json file that defines the mapping? |
|
well I can't think anything better...maybe I do want this to be plugin-able, but can't think of a good way to design the api. |
VictoriousRaptor
force-pushed
the
double-pin
branch
from
0c01c95 to
f673000
Compare
This comment has been minimized.
This comment has been minimized.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Pull Request Overview
Adds support for Double Pinyin in search by exposing new toggles in the UI, loading a JSON-based mapping table, and wiring it through settings, view models, and the PinyinAlphabet.
- UI: new switch and combo box for enabling double-pinyin and selecting a schema
- Settings & ViewModel: new
UseDoublePinyinandDoublePinyinSchemaproperties, with binding and localization - Core logic: JSON file of mappings, loading in
PinyinAlphabet, and a newTranslationMappingclass with unit tests
Reviewed Changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| SettingPages/Views/SettingsPaneGeneral.xaml | Added cards/toggles and schema selector |
| SettingsPaneGeneralViewModel.cs | Introduced UseDoublePinyin, schema list |
| Resources/double_pinyin.json | New mapping table for all supported schemas |
| Languages/en.xaml | Added localization for toggles and schemas |
| Flow.Launcher.csproj | Ensured JSON is copied to output |
| TranslationMapping.cs | New mapping utility with AddNewIndex/MapToOriginalIndex |
| TranslationMappingTest.cs | Unit tests for TranslationMapping |
| UserSettings/Settings.cs | Added settings, JSON serialization, enum |
| Infrastructure/StringMatcher.cs | Switched to ShouldTranslate predicate |
| Infrastructure/PinyinAlphabet.cs | Loads JSON, applies double-pinyin translation |
| Infrastructure/IAlphabet.cs | Updated interface to use ShouldTranslate |
Comments suppressed due to low confidence (3)
Flow.Launcher.Infrastructure/TranslationMapping.cs:29
- Method name 'endConstruct' doesn't follow .NET naming conventions; consider renaming to 'EndConstruct'.
public void endConstruct()
Flow.Launcher.Infrastructure/TranslationMapping.cs:29
- The behavior of
endConstructisn't covered by unit tests; consider adding tests to verify it prevents further calls toAddNewIndexafter construction.
public void endConstruct()
Flow.Launcher/Flow.Launcher.csproj:130
- Using 'Update' for a new resource entry may not include the JSON in the build; consider using 'Include' or verify that default globbing picks it up so it’s reliably copied under the Resources folder at runtime.
<None Update="Resources\double_pinyin.json">
onesounds
previously approved these changes
Jul 13, 2025
Jack251970
approved these changes
Jul 13, 2025
|
LGTM |
jjw24
removed
review in progress
This was referenced Jul 14, 2025
|
This feature is really awesome, thank you for your effort! |
10 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This pull request introduces a comprehensive set of changes to enhance Pinyin support in the application, including the addition of Double Pinyin (“双拼”) functionality, improved translation mappings, and related settings. The most significant updates include the implementation of the
IAlphabetinterface, the refactoring of thePinyinAlphabetclass, the introduction of Double Pinyin schemas, and updates to the user settings and localization files.UI in General Panel
Pinyin and Double Pinyin Enhancements:
IAlphabetinterface to define methods for language translation and determining translatability. (Flow.Launcher.Infrastructure/IAlphabet.cs)PinyinAlphabetclass to support Double Pinyin, utilize a cache for translations, and dynamically reload schemas based on user settings. (Flow.Launcher.Infrastructure/PinyinAlphabet.cs)Flow.Launcher.Infrastructure/UserSettings/Settings.cs) [1] [2]Translation Mapping Improvements:
TranslationMappingclass to simplify and optimize the mapping of original to translated indices. (Flow.Launcher.Infrastructure/TranslationMapping.cs)TranslationMappingto validate its functionality. (Flow.Launcher.Test/TranslationMappingTest.cs)Localization and Resource Updates:
Flow.Launcher/Languages/en.xaml)double_pinyin.jsonto project resources for schema definitions. (Flow.Launcher/Flow.Launcher.csproj)Codebase Simplifications:
StringMatcher.csto use the newShouldTranslatemethod fromIAlphabet. (Flow.Launcher.Infrastructure/StringMatcher.cs)IsAcronymandIsAcronymCountstatic for better encapsulation. (Flow.Launcher.Infrastructure/StringMatcher.cs) [1] [2]